-
Notifications
You must be signed in to change notification settings - Fork 21
docs: merge overview pages #732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fix capitalization of Actor changes to formatting streamlining prose
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #732 +/- ##
==========================================
- Coverage 81.98% 81.90% -0.08%
==========================================
Files 46 46
Lines 2681 2681
==========================================
- Hits 2198 2196 -2
- Misses 483 485 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -0,0 +1,163 @@ | |||
| --- | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was afraid that might happen. I think if we rename it to index.md it should not be separate openable section
| <Tabs> | ||
| <TabItem value="main.py" label="main.py" default> | ||
| <CodeBlock language="python">{ | ||
| `from apify import Actor | ||
| ${''} | ||
| async def main(): | ||
| async with Actor: | ||
| Actor.log.info('Actor input:', await Actor.get_input()) | ||
| await Actor.set_value('OUTPUT', 'Hello, world!')` | ||
| }</CodeBlock> | ||
| </TabItem> | ||
| <TabItem value="__main__.py" label="__main.py__"> | ||
| <CodeBlock language="python">{ | ||
| `import asyncio | ||
| import logging | ||
| ${''} | ||
| from apify.log import ActorLogFormatter | ||
| ${''} | ||
| from .main import main | ||
| ${''} | ||
| handler = logging.StreamHandler() | ||
| handler.setFormatter(ActorLogFormatter()) | ||
| ${''} | ||
| apify_logger = logging.getLogger('apify') | ||
| apify_logger.setLevel(logging.DEBUG) | ||
| apify_logger.addHandler(handler) | ||
| ${''} | ||
| asyncio.run(main())` | ||
| }</CodeBlock> | ||
| </TabItem> | ||
| </Tabs> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding some code comments?
docs/01_overview/index.mdx
Outdated
|
|
||
| ## Requirements | ||
|
|
||
| The Apify SDK requires Python version 3.8 or above to run Python Actors locally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs previously said that the required version is 3.10 or above. Is this updated version correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'll reach out and confirm the required version as client & sdk diverge here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on uv.lock seems like 3.10 or above is required @vdusek can you confirm so we can update accordingly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we test only on python 3.10+, afaik we dropped python 3.9 with the current v3 line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you , I'll update accordingly that 3.10 or above is required
vdusek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When running it locally, clicking "Docs" or "Get Started" redirects to
http://localhost:3000/sdk/python/docs/overview/introduction, which results in a 404.
|
Thanks @vdusek the landing pages will removed shortly (That is why we are doing this consolidation) but I changed the links so they work properly just in case. Secondary nav has been fixed as well |
rename consolidated page to index.mdx fix left side sidebar so it doesn't have 1 item in overview category, rather that overview is clickable page itself fix top side nav so Docs send to the new page fix link on landing page
Co-authored-by: Marcel Rebro <[email protected]>
docs/01_overview/index.mdx
Outdated
|
|
||
| The Apify SDK for Python is the official library for creating [Apify Actors](https://docs.apify.com/platform/actors) in Python. | ||
|
|
||
| ```python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it have make sense to have the code example as the first thing in the overview?
docs/01_overview/index.mdx
Outdated
|
|
||
| ### Creating Actors | ||
|
|
||
| To create and run Actors in Apify Console, refer to the [Console documentation](https://docs.apify.com/academy/getting-started/creating-actors#choose-your-template). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should refer to the platform docs, not academy imho
docs/01_overview/index.mdx
Outdated
|
|
||
| This will create a new folder called `my-first-actor`, download and extract the "Getting started with Python" Actor template there, create a virtual environment in `my-first-actor/.venv`, and install the Actor dependencies in it. | ||
|
|
||
| ### Running the Actor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could show example of the result (i.e. terminal output)
|
|
||
| This will activate the virtual environment in `.venv` (if no other virtual environment is activated yet), then start the Actor, passing the right environment variables for local running, and configure it to use local storages from the `storage` folder. | ||
|
|
||
| The Actor input, for example, will be in `storage/key_value_stores/default/INPUT.json`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite important information, and we should enhance it as many people struggle with this (they do not know where they should define INPUT.json locally)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps an admonition, let me try few different versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Admonition wouldn't work after all since I would have to. do it for whole paragraph. I opted to use bullet list (even though I hate them :D) to better flow with the whole doc, while making the content in question more scannable and accentuated. Let me know if you think any further changes are needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
change links shorten language declaration in codeblock restructure prose about apify run command
add gif with terminal run of apify create command fix mdlint issue around blank space near codeblock
|
@vdusek should we be worried about tests failing or do they have no bearing on docs change that we want to do? |
I'll restart it, but there's no need to worry - the (integration) tests rely only on the codebase, not the docs. |
marcel-rbro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍

No description provided.